feat(web): notification (and sound) when a thread finish - #7348
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:unvouched
|
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
Two consistency findings in the new Settings rows. Details inline.
Posted via Macroscope — UI Consistency
| <div className="flex w-full items-center gap-3 sm:w-52"> | ||
| <output | ||
| className="min-w-12 rounded-md bg-muted px-2 py-1 text-center font-mono text-xs font-medium tabular-nums text-foreground" | ||
| htmlFor="thread-completion-notification-volume-slider" | ||
| > | ||
| {volume}% | ||
| </output> | ||
| <input | ||
| aria-label="Notification volume" | ||
| className="settings-slider min-w-0 flex-1" | ||
| id="thread-completion-notification-volume-slider" | ||
| max={MAX_NOTIFICATION_VOLUME} | ||
| min={MIN_NOTIFICATION_VOLUME} | ||
| onChange={(event) => { | ||
| const nextVolume = Number(event.currentTarget.value); | ||
| if ( | ||
| !Number.isInteger(nextVolume) || | ||
| nextVolume < MIN_NOTIFICATION_VOLUME || | ||
| nextVolume > MAX_NOTIFICATION_VOLUME | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| primeNotificationChime(); | ||
| updateSettings({ threadCompletionNotificationVolume: nextVolume }); | ||
| if (previewTimerRef.current !== null) { | ||
| window.clearTimeout(previewTimerRef.current); | ||
| } | ||
| previewTimerRef.current = window.setTimeout(() => { | ||
| previewTimerRef.current = null; | ||
| playNotificationChime(nextVolume); | ||
| }, 400); | ||
| }} | ||
| step={10} | ||
| style={volumeSliderStyle} | ||
| type="range" | ||
| value={volume} | ||
| /> |
There was a problem hiding this comment.
This reconstructs the glass-opacity slider row (lines 1031-1058): identical output badge classes, identical settings-slider min-w-0 flex-1 input, and a second copy of the --settings-slider-progress / --settings-slider-fill-offset math, which encodes the half-thumb offset the .settings-slider CSS contract in index.css depends on. With two copies, a change to that fill math or to the badge geometry only lands on one slider. Consider extracting a named control next to SettingsRow in settingsLayout.tsx (value, min, max, step, aria-label, id, onChange) and rendering both rows through it, keeping call-site-only concerns such as the chime preview timer here.
Posted via Macroscope — UI Consistency
| <Switch | ||
| checked={enabled} | ||
| onCheckedChange={(checked) => { | ||
| if (checked) { | ||
| enable(); | ||
| return; | ||
| } | ||
| updateSettings({ threadCompletionNotifications: false }); | ||
| }} | ||
| aria-label="Thread completion notifications" | ||
| /> |
There was a problem hiding this comment.
When permission is denied or unsupported, enable() returns early, so this switch accepts the click, reports no state change, and snaps back — an interactive-looking control that cannot do anything. Switch already carries a disabled contract (data-disabled:cursor-not-allowed, reduced opacity, no keyboard activation), and the row's status text already explains why. Consider disabling it only while it is off and permission cannot be granted, so the documented case (permission revoked after opting in) still leaves a switch the user can turn off.
| <Switch | |
| checked={enabled} | |
| onCheckedChange={(checked) => { | |
| if (checked) { | |
| enable(); | |
| return; | |
| } | |
| updateSettings({ threadCompletionNotifications: false }); | |
| }} | |
| aria-label="Thread completion notifications" | |
| /> | |
| <Switch | |
| checked={enabled} | |
| disabled={!enabled && (permission === "denied" || permission === "unsupported")} | |
| onCheckedChange={(checked) => { | |
| if (checked) { | |
| enable(); | |
| return; | |
| } | |
| updateSettings({ threadCompletionNotifications: false }); | |
| }} | |
| aria-label="Thread completion notifications" | |
| /> |
Posted via Macroscope — UI Consistency
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6c24b9c89ad9718b207759950851abf0a40e9561. Configure here.
| showNotification, | ||
| useNotificationPermission, | ||
| } from "../../notificationPermission"; | ||
| import { playNotificationChime, primeNotificationChime } from "../../notificationChime"; |
There was a problem hiding this comment.
Restore skips notification settings
Medium Severity
The new General settings threadCompletionNotifications, threadCompletionNotificationSound, and threadCompletionNotificationVolume are omitted from changedSettingLabels and the restoreDefaults updateSettings payload. Changing them never dirties Restore defaults, and Restore leaves them unchanged while resetting neighboring General rows.
Reviewed by Cursor Bugbot for commit 6c24b9c89ad9718b207759950851abf0a40e9561. Configure here.
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR introduces a complete new feature (thread completion notifications with audio chimes), adding new components, browser API integrations, settings schema changes, and ~860 lines of new code. New features introducing user-facing behavior warrant human review. Additionally, unresolved Medium-severity findings identify missing restore-defaults handling and a documentation inaccuracy. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
jaakkoob
changed the title
|
Huge +1 from me! I’d be genuinely excited to have completion sounds in T3 Code—being able to switch away during a long-running thread and hear when it’s ready would be fantastic. The opt-in sound and volume controls proposed here look especially nice. 🔔 |
Nothing tells you a thread is done unless you are looking at it, so long runs mean polling the tab. Web and desktop have no completion notification at all; only mobile does, and that path needs the relay. Adds an opt-in client setting that raises a native notification, with an optional synthesized chime, when a thread's latest turn completes. It announces a transition rather than a state, so it needs no clock of its own: a thread is news only once this client has seen it unfinished and then finished. A thread seen for the first time — on load, on reconnect, or when an environment joins late — is recorded silently, which is what keeps a page load from announcing the user's whole history. Turns that were interrupted or errored are not announced, and neither is a thread whose `backgroundLiveness` is still `working`, so delegating to subagents does not read as finishing. The watcher lives in the app shell so a turn finishing while the user is in Settings still reaches them, and subscribes to thread updates only while the setting is on and permission is granted. Web and desktop only; desktop inherits it through the web bundle. Mobile is untouched — it already has relay-backed push. Notifications need an open tab by construction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
6c24b9c to
d19d671
Compare
|
|
||
| Notifications are delivered by the app itself, so they arrive while T3 Code is open in a tab or in | ||
| the desktop app, including when it is in the background. A closed tab receives nothing. On iOS and | ||
| Android, the mobile app delivers its own push notifications instead. |
There was a problem hiding this comment.
🟡 Medium user/notifications.md:21
Android users are told they will receive completion push notifications, but the mobile app does not deliver them because requestAgentNotificationPermission and SettingsRouteScreen support only iOS. Update this sentence to mention iOS only.
| Android, the mobile app delivers its own push notifications instead. | |
| On iOS, the mobile app delivers its own push notifications instead. |
🤖 Copy this AI Prompt to have your agent fix this:
In file @docs/user/notifications.md around line 21:
Android users are told they will receive completion push notifications, but the mobile app does not deliver them because `requestAgentNotificationPermission` and `SettingsRouteScreen` support only iOS. Update this sentence to mention iOS only.
|
Its honestly kinda strange we dont have alerts yet. T3 is such a cool app, but some sort of sound notifications when a thread needs input or is done would improve it by a mile. |
|
@t3dotgg @maria-rcks can someone please review this? need alerts and could be turned off by default in settings so only people need it can turn it on, happy to contribute and make this better in this PR itself but please review |
|
This would be super useful, feels like missing basic functionality 🙏 |
|
Another +1 here, this is a no-brainer without any downside and very much needed |
|
Superseded by #11481 (opt-in thread notifications and sounds on main). Closing as superseded. |
7 participants
We have no way to tell a thread is done right now other than by looking at it. I'm always looking at the t3 code tab every 5 minutes or so to check if a thread is done, because there is no other way of knowing. That's why I think we should add notification. I tested the web implementation in chromium browser (helium), firefox browser (zen) and safari.
Settings → General gains three rows, revealed progressively: Completion notifications (Test + switch), Notification sound, Notification volume.
Written by Claude Opus 5 in Claude Code; the Test button and volume control were implemented by GPT-5.6-sol via Codex CLI, which also reviewed the branch twice.
Note
Low Risk
Client-only UX: browser notifications, local settings, and Web Audio; no auth, server, or data-model changes beyond new optional client settings fields.
Overview
Adds completion notifications so users get a system alert when a thread’s turn finishes while the app is open (tab or desktop), without polling the UI.
Settings → General adds three linked controls: enable notifications (with permission prompt and Test), optional notification sound, and a volume slider with live chime preview. New client settings (
threadCompletionNotifications, sound, volume) are defined in contracts and wired through desktop settings tests.A
ThreadCompletionNotificationswatcher lives in the app shell (not the chat route) so completions are detected from any screen and remounts don’t reset state. It only subscribes to thread data when notifications are on and permission is granted.deriveThreadCompletionNotificationsfires on transitions (running → completed), skips first-seen threads on load, ignores interrupted/error/archived turns, waits for session idle and for subagentbackgroundLivenessto clear, and uses turn-scoped notification tags. Clicks focus the window and navigate to the thread.Supporting pieces:
notificationPermission(shared permission store +showNotification),notificationChime(Web Audio two-note chime with gesture priming), unit tests for logic and gain, settings search entries, and user docs atdocs/user/notifications.md.Reviewed by Cursor Bugbot for commit d19d671. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add OS notifications and optional chime sound when a thread turn completes
ThreadCompletionNotificationscomponent mounted in the app shell that watches all threads and fires an OS notification when a turn transitions from running to completed.playNotificationChimeandchimeGainForVolumein notificationChime.ts.useNotificationPermissionhook andshowNotificationutil in notificationPermission.ts to manage permission state across the app lifecycle and display click-to-focus OS notifications.ClientSettingsSchemawiththreadCompletionNotifications,threadCompletionNotificationSound, andthreadCompletionNotificationVolume(default off, sound on, volume 70).📊 Macroscope summarized d19d671. 10 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.